Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dotnet): Fix property set for nested Dictionaries #736

Merged
merged 2 commits into from
Aug 26, 2019

Conversation

assyadh
Copy link
Contributor

@assyadh assyadh commented Aug 23, 2019

The runtime fails to cast properties when a Dictionary<string, object> is actually a Dictionary<string, Dictionary<string, object>>

Example:

            var launchTemplateData = new CfnLaunchTemplate.LaunchTemplateDataProperty
            {
                InstanceType = "t3.medium",
                Monitoring = new CfnLaunchTemplate.MonitoringProperty { Enabled = true }
            };

            var cfnLaunchTemplate = new CfnLaunchTemplate(stack, "template", new CfnLaunchTemplateProps
            {
                LaunchTemplateData = launchTemplateData,
                LaunchTemplateName = "template"
            });

In this example, the LaunchTemplateData property is a Dictionary<string, object>. However, the 'Monitoring' key is also a Dictionary<string, object>. This was not supported by the runtime and would fail.

Fixes aws/aws-cdk#2496

Cdk synth output after the fix:

Resources:
  template:
    Type: AWS::EC2::LaunchTemplate
    Properties:
      LaunchTemplateData:
        InstanceType: t3.medium
        Monitoring:
          Enabled: true
      LaunchTemplateName: template
    Metadata:
      aws:cdk:path: HamzaStack/template

Also added a unit test to cover complex properties.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@assyadh assyadh requested a review from a team as a code owner August 23, 2019 21:10
@mergify
Copy link
Contributor

mergify bot commented Aug 23, 2019

The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged.
[Conventional Commits]: https://www.conventionalcommits.org

@assyadh assyadh changed the title fix(dotnet) Fix property set for complex Dictionaries fix(dotnet): Fix property set for complex Dictionaries Aug 23, 2019
@RomainMuller RomainMuller changed the title fix(dotnet): Fix property set for complex Dictionaries fix(dotnet): fix property set for complex Dictionaries Aug 26, 2019
@RomainMuller RomainMuller changed the title fix(dotnet): fix property set for complex Dictionaries fix(dotnet): Fix property set for nested Dictionaries Aug 26, 2019
@mergify
Copy link
Contributor

mergify bot commented Aug 26, 2019

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Aug 26, 2019
@mergify mergify bot merged commit 04bab47 into master Aug 26, 2019
@mergify mergify bot added pr/ready-to-merge This PR is ready to be merged. and removed pr/ready-to-merge This PR is ready to be merged. labels Aug 26, 2019
@mergify mergify bot deleted the hamzaad/fix-2496 branch August 26, 2019 07:02
@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LaunchTemplateDataProperty/CfnLaunchTemplate_ fails with any complex property set
3 participants